Add defaultEventSource to WorkflowApplication#1556
Conversation
|
@matheusandre1 you must sign your commits with |
Sorry, I always forget about that. |
fjtirado
left a comment
There was a problem hiding this comment.
@matheusandre1
Lets make default event source a WorkflowValueResolver, so it returns an URI from the WorkflowContext, TaskContext and Model
bd2d409 to
37dd274
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves CloudEvent emission defaults by introducing an application-level default source resolver and by ensuring emitted events always have a time value, aligning emitted events more closely with the Serverless Workflow spec requirements discussed in #1554.
Changes:
- Added
defaultEventSourcetoWorkflowApplication(configurable via builder, with a sensible default resolver). - Updated
EmitExecutorto defaultsourcevia the application’s resolver and to always settime(defaulting toOffsetDateTime.now()). - Added test coverage (including a YAML-based workflow sample) verifying defaulting and precedence behavior for
source,time, andid.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| impl/test/src/test/resources/workflows-samples/emit-default-source.yaml | Adds a YAML workflow sample used to verify default CloudEvent source behavior when loading workflows from YAML. |
| impl/test/src/test/java/io/serverlessworkflow/impl/test/EmitExecutorTest.java | Adds tests covering source defaulting/override precedence, and time/id defaults for emitted events. |
| impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java | Introduces defaultEventSource on the application and builder APIs to configure it, with a default resolver. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/EmitExecutor.java | Switches emitted event source defaulting to use WorkflowApplication.defaultEventSource() and defaults time to now. |
| impl/core/src/main/java/io/serverlessworkflow/impl/events/EmitSourceResolver.java | Adds the default resolver implementation that builds source from application id + workflow definition id. |
| impl/core/src/main/java/io/serverlessworkflow/impl/events/CloudEventUtils.java | Updates the static fallback source() URI value to a more meaningful identifier. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@matheusandre1 when generating the |
Signed-off-by: Matheus André <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java:413
withDefaultEventSource(URI)acceptsnulland wraps it into a resolver that returnsnull. That prevents the defaultEmitSourceResolverfallback from being applied and will later passnullintoCloudEventBuilder.withSource(...)(likely causing a runtime NPE/IAE during emit). Fail fast by rejectingnullhere.
public Builder withDefaultEventSource(URI defaultEventSource) {
return withDefaultEventSource((workflow, task, model) -> defaultEventSource);
}
impl/core/src/main/java/io/serverlessworkflow/impl/executors/EmitExecutor.java:109
defaultEventSource().apply(...)is used directly as the CloudEvent source. If a custom resolver returnsnull(or a caller previously passed a null URI), this will propagate intoceBuilder.withSource(...)and fail at runtime with an opaque NPE. Enforce a non-null resolved source with a clear message.
workflow
.definition()
.application()
.defaultEventSource()
.apply(workflow, taskContext, taskContext.input())));
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it: Closes: #1554
Special notes for reviewers:
Additional information (if needed):